Skip to content

fix(common/core/web): error from early fat-finger termination due to OS interruptions 🍒#5491

Merged
jahorton merged 6 commits intostable-14.0from
fix/common/core/web/cherry/empty-alternates
Jul 28, 2021
Merged

fix(common/core/web): error from early fat-finger termination due to OS interruptions 🍒#5491
jahorton merged 6 commits intostable-14.0from
fix/common/core/web/cherry/empty-alternates

Conversation

@jahorton
Copy link
Contributor

@jahorton jahorton commented Jul 27, 2021 •
edited
Loading

A 🍒-pick of #5479.

User Testing

@keymanapp/testers

It's literally the same set of tests that was used for #5479.

Much of this is taken from standard KMW acceptance testing and adapted toward the aspects of KMW modified by this PR, though we'll only worry about testing against a single platform here.

Platform: Chrome emulation / Android

Ensure that you see the "Console" tab while performing these tests.

  • TEST_CRITICAL: If any errors (red text entries in the "Console" area) or warnings appear during these tests, fail this test entry and report a screenshot of them.
    • Ignore anything seen elsewhere mentioning "Issues" or within a small "Issues" tab - there are a few entries we know about, but they're not affected by this PR. I'm looking for "Console" log messages.

Console tab:

image

It should resemble Windows' CMD prompt and the macOS Terminal.

Utilize the "Test unminified Keymanweb" testing page to ensure the following:

  • TEST_1: KMW properly handles input to the controls.
  • Attempt to add the following keyboards:
    • TEST_2: Using "Add a keyboard by keyboard name", add: sil_ipa. (For our currently-deprecated IPA keyboard.)
      • TEST_3: ŋ should be a subkey of n - ensure it works.
    • TEST_4: Using "Add a keyboard by keyboard name", add: khmer_angkor
      • TEST_5: Type in the following sequence: ស, ុ, ្រ (subkey of រ), ក. You should see ស្រុក.
      • TEST_6: Continuing from the last test, hit backspace in sequence. As this is a reorder (keyboard) rule test, you should see:
        • ស្រុ
        • ស្រ
        • ស
    • TEST_7: Using "Add a keyboard by language name(s)", add: spanish.

Utilize the "Prediction - robust testing" testing page for the following:

  • Swap to the "English - EuroLatin (SIL)" keyboard.
    • TEST_8: Use long-press . to output ' and then press e. The two characters should not combine.
    • TEST_9: Ensure that long-press p and SHIFT + long-press g displays properly and that the subkeys produce the expected output.
    • TEST_10: Type the following sequence, pressing near the center of the key each time:
      • L (shift layer)
      • k (default layer)
      • v (default layer)
      • Expected result: you should see suggestions for Love and Live, along with one other random suggestion.

@jahorton jahorton added this to the A15S10 milestone Jul 27, 2021
@github-actions github-actions bot added common/ common/core/ common/web/ cherry-pick Change already merged into another (stable) branch fix web/ labels Jul 27, 2021
@jahorton
Copy link
Contributor Author

jahorton commented Jul 27, 2021 •
edited
Loading

Huh... git rebase -i didn't auto-cherry / include the documentation comment that was applied by code suggestion. Odd.

mcdurdin
mcdurdin approved these changes Jul 27, 2021
View reviewed changes
Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Any sanity user testing we can do?

@jahorton
Copy link
Contributor Author

jahorton commented Jul 27, 2021

LGTM. Any sanity user testing we can do?

"Can do?" We can always run the same tests from the PR against master. There's no reliable way to set up an actual repro to test, though.

@mcdurdin
Copy link
Member

mcdurdin commented Jul 27, 2021

There's no reliable way to set up an actual repro to test, though.

I'm looking more for regression tests -- make sure nothing else gets broken. I'm fairly confident that this is true, but can you add a user test for Makara similar to the one we did for master?

@jahorton
Copy link
Contributor Author

jahorton commented Jul 27, 2021 •
edited
Loading

... but can you add a user test for Makara similar to the one we did for master?

Gave it the ol' CTRL+C -> CTRL+V sequence.

@jahorton jahorton mentioned this pull request Jul 27, 2021
Merged
33 tasks
@MakaraSok
Copy link

MakaraSok commented Jul 28, 2021 •
edited by jahorton
Loading

User Testing

@keymanapp/testers

Platform: Chrome emulation / Android

Build and test on Windows 10 Pro 21H1, Chrome 92.0.4515.107 (x64) - Pixel 2

  • TEST_CRITICAL: FAILED - No errors (red text entries in the "Console" area) or warnings appear during "Test unminified Keymanweb". There is one thing though during "Prediction - robust testing".

This shows up when switching the keyboard to EuroLatin (SIL).
image

Utilize the "Test unminified Keymanweb" testing page to ensure the following:

  • TEST_1: PASSED - the OSK can be used as usual
  • Attempt to add the following keyboards:
    • TEST_2: PASSED - no issue when adding sil_ipa using "Add a keyboard by keyboard name".
      • TEST_3: PASSED - ŋ is one of the subkeys of n - it works as intended.
    • TEST_4: PASSED - no issue when adding khmer_angkor using "Add a keyboard by keyboard name".
      • TEST_5: PASSED - type in ស ុ ្រ (subkey of រ) ក and the output indeed is ស្រុក.
      • TEST_6: PASSED - Continuing from the last test, hit backspace in sequence. As this is a reorder (keyboard) rule test, each backspace indeed shows:
        • ស្រុ
        • ស្រ
        • ស
    • TEST_7: PASSED - Using "Add a keyboard by language name(s)", add: spanish and a Spanish keyboard shows up on the keyboard menu off of the globe key

Utilize the "Prediction - robust testing" testing page for the following:

  • Swap to the "English - EuroLatin (SIL)" keyboard.
    • TEST_8: PASSED - Use long-press . to output ' and then press e. The two characters do not combine per expectation.
    • TEST_9: PASSED - long-press p and SHIFT + long-press g displays properly and that the subkeys produce the expected output.
    • TEST_10: PASSED - Type the following sequence, pressing near the center of the key each time:
      • L (shift layer)
      • k (default layer)
      • v (default layer)
      • Expected result: the expected suggestions for Love and Live, along with one other random suggestion are shown.

@jahorton
Copy link
Contributor Author

jahorton commented Jul 28, 2021

Eh, that's fine. (The outputContext missing message. Not ideal, but it's definitely not branch-local.) And it's "yellow," not red, so it's not worth blocking on.

@jahorton jahorton merged commit 7f12d5a into stable-14.0 Jul 28, 2021
@jahorton jahorton deleted the fix/common/core/web/cherry/empty-alternates branch July 28, 2021 10:06
@MakaraSok
Copy link

MakaraSok commented Aug 3, 2021

User Testing [retest on Android for good measure]

@keymanapp/testers
@darcywong00 @mcdurdin

Platform: Android 6.0.1 emulated on Nexus 5

Test build #14.0.280-test at 2 Aug 01:52

Utilize the "Test unminified Keymanweb" testing page to ensure the following:

  • TEST_1: PASSED - the OSK can be used as usual
  • Attempt to add the following keyboards:
    • TEST_2: PASSED - no issue when adding sil_ipa using "Add a keyboard by keyboard name".

    successfully installed this from Settings > Install Keyboard or Dictionary > Install from keyman.com > sil_ipa

    • TEST_3: PASSED - ŋ is one of the subkeys of n - it works as intended.
    • TEST_4: PASSED - no issue when adding khmer_angkor using "Add a keyboard by keyboard name".

    successfully installed this from Settings > Install Keyboard or Dictionary > Install from keyman.com > khmer_angkor

    • TEST_5: PASSED - type in ស ុ ្រ (subkey of រ) ក and the output indeed is ស្រុក.
    • TEST_6: PASSED - Continuing from the last test, hit backspace in sequence. As this is a reorder (keyboard) rule test, each backspace indeed shows:
      • ស្រុ
      • ស្រ
      • ស
    • TEST_7: PASSED - Using "Add a keyboard by language name(s)", add: spanish and a Spanish keyboard shows up on the keyboard menu off of the globe key

    Tap on the plus sign in Installed Languages>type in the search box Spanish>tap to install EuroLatin (SIL) keyboard for Spanish language>successfully installed.

Utilize the "Prediction - robust testing" testing page for the following:

  • Swap to the "English - EuroLatin (SIL)" keyboard.
    • TEST_8: PASSED - Use long-press . to output ' and then press e. The two characters do not combine per expectation.
    • TEST_9: PASSED - long-press p and SHIFT + long-press g displays properly and that the subkeys produce the expected output.
    • TEST_10: FAILED - Type the following sequence, pressing near the center of the key each time:
      • L (shift layer)
      • k (default layer)
      • v (default layer)
      • Expected result: the expected suggestions for Love and Live, along with one other random suggestion are shown.
      • Actual result: no suggestion given in the banner. See the screenshot below.
      image

@MakaraSok
Copy link

MakaraSok commented Aug 4, 2021

Retested for good measure using the stable build #14.0.280-test at 2 Aug 01:52.

Confirm that all tests have PASSED. Things are working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcdurdin mcdurdin mcdurdin approved these changes

Assignees

No one assigned

Labels

cherry-pick Change already merged into another (stable) branch common/web/ common/ fix has-user-test web/

Projects

None yet

Milestone

A15S10

Development

Successfully merging this pull request may close these issues.

3 participants

@jahorton @mcdurdin @MakaraSok